Skip to content

feat: replace render-time text wrapping with browser-native wrapping#191

Merged
martin-mfg merged 8 commits into
stats-organization:masterfrom
hyperfinitism:fix/use-foreign-obj
May 6, 2026
Merged

feat: replace render-time text wrapping with browser-native wrapping#191
martin-mfg merged 8 commits into
stats-organization:masterfrom
hyperfinitism:fix/use-foreign-obj

Conversation

@hyperfinitism
Copy link
Copy Markdown

@hyperfinitism hyperfinitism commented Apr 27, 2026 •
edited
Loading

This PR uses foreignObject with CSS line-clamp to let the browser handle text wrapping natively instead of manually wrapping on the server. This provides better font-aware wrapping while keeping server-side line estimation for SVG height calculation.

Fixes anuraghazra#4862.

Example card

hyperfinitism-sample-repo

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 27, 2026

@hyperfinitism is attempting to deploy a commit to the martin-mfg's projects Team on Vercel.

A member of the Team first needs to authorize it.

@hyperfinitism hyperfinitism mentioned this pull request Apr 27, 2026
Closed
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 28, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
github-stats-extended-backend Ready Ready Preview, Comment May 2, 2026 10:30am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
github-stats-extended-frontend Ignored Ignored May 2, 2026 10:30am

martin-mfg
martin-mfg requested changes Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@martin-mfg martin-mfg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Please ignore the failing "Backend E2E test" pipeline. It determined that this PR generates different output than the master branch, which is expected here.

Currently the changes in this PR lead to visual changes in generated cards regardless of line wrapping. Especially the line height is increased. Could you please ensure there are no visual changes apart from the line wrapping? Maybe it makes sense to simply copy the HTML code which GitHub itself uses in the original repo pins on user profiles?

Doing this would also raise the question if it makes sense to use foreignObject for the whole content of gist/pin cards. I.e. not only the description, but also the title, star count, etc. What do you think?

Comment thread packages/core/src/cards/repo.js Outdated
Comment thread packages/core/src/cards/repo.js
Comment thread packages/core/src/cards/repo.js
: ""
}

<text class="description" x="${X_OFFSET}" y="-5">
Copy link
Copy Markdown
Member

@martin-mfg martin-mfg Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does removing the "-5" here not change the layout of the final card?

Copy link
Copy Markdown
Author

@hyperfinitism hyperfinitism Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly speaking it does; the new position depends on the browser's HTML/font metrics rather than the SVG baseline, so the first-line baseline can drift by about < 1px. In practice the visual top of the description lines up at the same place and the appearance is nearly identical.

Comment thread packages/core/src/common/render.js Outdated
@hyperfinitism hyperfinitism force-pushed the fix/use-foreign-obj branch 2 times, most recently from f9af708 to 0c6113b Compare April 28, 2026 14:16
@hyperfinitism
feat: replace render-time text wrapping with browser-native wrapping
7da1078 
Use `foreignObject` with CSS line-clamp to let the browser handle text
wrapping natively instead of manually wrapping on the server.
This provides better font-aware wrapping while keeping server-side line
estimation for SVG height calculation.

Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>
@hyperfinitism
Copy link
Copy Markdown
Author

hyperfinitism commented Apr 28, 2026

Doing this would also raise the question if it makes sense to use foreignObject for the whole content of gist/pin cards. I.e. not only the description, but also the title, star count, etc. What do you think?

While this approach maintains the greatest visual consistency, it has the following problems:

  • A core feature of github-readme-stats is letting users override title_color, text_color, bg_color, presets, etc. With our own SVG markup we own every fill/color and substitute cleanly. When using GitHub templates, you need to completely redesign the theme.
  • The current cards are SVG with inlined Octicons paths, so they render identically anywhere, e.g. offline previews. Pulling in GitHub-hosted icons or HTML scaffolds would make the cards depend on GitHub's CDN and DOM structure and break those uses.
  • Using foreignObject for a single multi-line text node is a small, contained departure from pure SVG. Embedding GitHub's full HTML pin would, in effect, make this an HTML-snippet generator rather than an SVG generator. This would be a fairly fundamental change.

@martin-mfg
Copy link
Copy Markdown
Member

martin-mfg commented May 1, 2026

While this approach maintains the greatest visual consistency, it has the following problems:

I agree we should not further extend the usage of foreignObject here.

I have made some changes based on your branch:

  • introduce parameter browser_rendering to switch between server-side rendering and client-side rendering
    Default is server-side, because in case our server-side estimation of lines used by browser-side rendering is wrong, then the card will look really bad. This should not be the default.
  • adapt wrapTextMultiline to use the new and improved wrapping logic
  • included measureText improvements from use pixel-width-based word wrapping #189
  • modify whitespace handling

I have pushed them as a new branch. I tried pushing them directly to your fork, but it seems this GitHub bug prevent this.

What I still want to do is to visually compare new card output to card output from master branch and make sure visual changes are mostly limited to line wrapping. Including on different operating systems and in different browsers. Then I'd like to merge this PR once we're about to release a new major version of github-stats-extended.

But first, do you have any comments on my code changes?

@martin-mfg martin-mfg mentioned this pull request May 1, 2026
Closed
@hyperfinitism
Copy link
Copy Markdown
Author

hyperfinitism commented May 1, 2026

I think it is a good idea to allow users to switch between rendering modes. I have cherry-picked all of your commits from the branch.

@martin-mfg martin-mfg merged commit e43e55c into stats-organization:master May 6, 2026
7 of 8 checks passed
@hyperfinitism hyperfinitism deleted the fix/use-foreign-obj branch May 6, 2026 09:18
@hyperfinitism hyperfinitism mentioned this pull request May 7, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martin-mfg martin-mfg martin-mfg requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Repository Pin/Gist Card Description Overflow

2 participants

@hyperfinitism @martin-mfg